Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update build script to publish to maven local #596

Merged
merged 3 commits into from
Oct 27, 2022
Merged

Update build script to publish to maven local #596

merged 3 commits into from
Oct 27, 2022

Conversation

jmazanec15
Copy link
Member

Description

Updates build script to publish zip to local maven. This allows plugins like neural search to pick up the zip as a dependency.

Fixes bug in build script where group was being set to org.opensearch as opposed to org.opensearch.plugin.

Publish to maven passed:

You can use '--warning-mode all' to show the individual deprecation warnings and determine if they come from your own scripts or plugins.

See https://docs.gradle.org/7.5/userguide/command_line_interface.html#sec:command_line_warnings

Execution optimizations have been disabled for 1 invalid unit(s) of work during this build to ensure correctness.
Please consult deprecation warnings for more details.

BUILD SUCCESSFUL in 16s
38 actionable tasks: 38 executed

Tried with variety
./gradlew :publishToMavenLocal -Dbuild.snapshot=true -Dbuild.version_qualifier=rc1
./gradlew :publishToMavenLocal -Dbuild.snapshot=true
./gradlew :publishToMavenLocal -Dbuild.snapshot=false

ls ~/.m2/repository/org/opensearch/plugin/opensearch-knn/
3.0.0.0				3.0.0.0-SNAPSHOT		3.0.0.0-rc1-SNAPSHOT		maven-metadata-local.xml

Issues Resolved

opensearch-project/neural-search#31

Check List

  • Commits are signed as per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@jmazanec15
Update build to publish to maven local
2d0c916 
Updates build script to publish zip to local maven. This allows plugins
like neural search to pick up the zip as a dependency.

Fixes bug in build script where group was being set to org.opensearch as
opposed to org.opensearch.plugin.

Signed-off-by: John Mazanec <jmazane@amazon.com>
@jmazanec15 jmazanec15 requested a review from a team October 26, 2022 20:57
@jmazanec15 jmazanec15 added backport 2.x Infrastructure Changes to infrastructure, testing, CI/CD, pipelines, etc. labels Oct 26, 2022
@codecov-commenter
Copy link

Codecov Report

Merging #596 (2d0c916) into main (5bb7a3f) will decrease coverage by 0.25%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##               main     #596      +/-   ##
============================================
- Coverage     84.77%   84.51%   -0.26%     
+ Complexity     1059     1054       -5     
============================================
  Files           149      149              
  Lines          4301     4301              
  Branches        382      382              
============================================
- Hits           3646     3635      -11     
- Misses          480      489       +9     
- Partials        175      177       +2
Impacted Files Coverage Δ
...va/org/opensearch/knn/index/KNNCircuitBreaker.java 60.00% <0.00%> (-20.00%) ⬇️
...ain/java/org/opensearch/knn/index/KNNSettings.java 80.88% <0.00%> (-2.21%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

This was referenced Oct 26, 2022
Closed
Closed
Closed
peterzhuamazon
peterzhuamazon reviewed Oct 26, 2022
View reviewed changes
build.gradle Outdated
@@ -53,7 +53,7 @@ ext {

allprojects {

group = 'org.opensearch'
group = 'org.opensearch.plugin'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not ideal because we set group=org.opensearch so jars can be pushed to org/opensearch/<>.
We then set groupId=org.opensearch.plugin so zips can be pushed to org/opensearch/plugin/<>.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For k-NN, we dont puish any jars at the moment.

Copy link
Contributor

@reta reta Oct 26, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jmazanec15 you could keep group=org.opensearch there and use groupId=org.opensearch.plugin for ZIPs, it will fix the issue:

   > Invalid publication 'nebula': supplied groupId (org.opensearch) does not match value from POM file (org.opensearch.plugin). Cannot edit groupId directly in the POM file.

Copy link
Member Author

@jmazanec15 jmazanec15 Oct 26, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@reta I believe this is what we do now:

  1. https://github.com/opensearch-project/k-NN/blob/main/build.gradle#L56
  2. https://github.com/opensearch-project/k-NN/blob/main/build.gradle#L91

I think I can leave as is, but change build script to use:

./gradlew publishPluginZipPublicationToMavenLocal

@jmazanec15
Switch to use publishPluginZipPublicationToMavenLocal
b0ae128 
Signed-off-by: John Mazanec <jmazane@amazon.com>
@jmazanec15 jmazanec15 requested review from reta, naveentatikonda and martin-gaievski and removed request for reta October 27, 2022 00:00
@jmazanec15
Add opensearch version to publishZip
2a30817 
Signed-off-by: John Mazanec <jmazane@amazon.com>
@jmazanec15 jmazanec15 merged commit 5323699 into opensearch-project:main Oct 27, 2022
opensearch-trigger-bot bot pushed a commit that referenced this pull request Oct 27, 2022
@jmazanec15 @github-actions
Update build script to publish to maven local (#596)
f9139d7 
Updates build script to publish zip to local maven. This allows plugins
like neural search to pick up the zip as a dependency.

Signed-off-by: John Mazanec <jmazane@amazon.com>
(cherry picked from commit 5323699)
@opensearch-trigger-bot opensearch-trigger-bot bot mentioned this pull request Oct 27, 2022
Merged
@heemin32 heemin32 added the v2.4.0 'Issues and PRs related to version v2.4.0' label Nov 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@naveentatikonda naveentatikonda naveentatikonda approved these changes

@peterzhuamazon peterzhuamazon peterzhuamazon approved these changes

@reta reta reta approved these changes

@martin-gaievski martin-gaievski Awaiting requested review from martin-gaievski martin-gaievski is a code owner

Assignees
No one assigned
Labels
backport 2.x Infrastructure Changes to infrastructure, testing, CI/CD, pipelines, etc. v2.4.0 'Issues and PRs related to version v2.4.0'
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@jmazanec15 @codecov-commenter @reta @peterzhuamazon @naveentatikonda @martin-gaievski @heemin32